Handle unauthenticated users when creating a new workspace #3578
Conversation
| class PublicWorkspaceNewView extends React.PureComponent { | ||
| componentDidMount() { | ||
| Onyx.merge(ONYXKEYS.SESSION, {redirectToWorkspaceNewAfterSignIn: true}); | ||
| this.props.navigation.replace(SCREENS.HOME); |
There was a problem hiding this comment.
Note: Had to use this.props.navigation instead of src/libs/Navigation/Navigation -> navigate because navigationRef didn't exist immediately when navigating to /workspace/new
There was a problem hiding this comment.
Nice, that's interesting and I'd be curious to know more about why that is so we can standardize on this.props.navigtation or Navigation.navigate() and not have to use a combination of them. But this works ok for now I think. Maybe we can create a follow up to figure it out at some point.
| }, | ||
| Onyx.merge(ONYXKEYS.SESSION, { | ||
| shouldShowComposeInput: true, | ||
| ..._.pick(data, 'authToken', 'accountID', 'email'), |
There was a problem hiding this comment.
I can see why this needed to be switched from multiSet() to merge(), but curious if there are reasons why we were using the set behavior over the merge behavior and if anything unexpected will come from this change (I'm not really sure so maybe it's a NAB - but sort of unclear why we used one over the other in the first place).
There was a problem hiding this comment.
Yeah I was thinking about that, my assumption is that since it was done 7 months ago, we just didn't "need to" do .merge yet. I can keep an eye on issues related to Session storage in the next few weeks just in case
| class PublicWorkspaceNewView extends React.PureComponent { | ||
| componentDidMount() { | ||
| Onyx.merge(ONYXKEYS.SESSION, {redirectToWorkspaceNewAfterSignIn: true}); | ||
| this.props.navigation.replace(SCREENS.HOME); |
There was a problem hiding this comment.
Nice, that's interesting and I'd be curious to know more about why that is so we can standardize on this.props.navigtation or Navigation.navigate() and not have to use a combination of them. But this works ok for now I think. Maybe we can create a follow up to figure it out at some point.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.68-5🚀
|
|
🚀 Deployed to production in version: 1.0.73-3🚀
|
Details
If an unauthenticated user navigates to https://expensify.cash/workspace/new, we redirect them to the signin page and, after signing in, send them back to https://expensify.cash/workspace/new.
cc @marcaaron
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/166528
Tests
QA Steps
Tested On
Screenshots
Web
Screen.Recording.2021-06-14.at.7.27.09.PM.mov
Mobile Web
Screen.Recording.2021-06-14.at.7.29.10.PM.mov
Desktop
N/A
iOS
N/A
Android
N/A